Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

flaky-test-check has problems running @Nested tests (e.g. TestLegacyReplicationManager).

Setting test class name:

  • TestLegacyReplicationManager: no tests are run
  • TestLegacyReplicationManager$Misreplicated: fails in prepare-job step, file is not found; also would require separate workflow run per nested class
  • TestLegacyReplicationManager*: file found, tests executed OK (but twice), but artifact upload fails (artifact name includes test class name): The artifact name is not valid: ... Contains the following character: Asterisk *

Changes in this PR:

  1. Remove the step that required the test class to be defined in a similarly named file.
    • junit.sh can execute tests in any module
    • misspelled test class results in failed run (implemented in HDDS-10080)
  2. Avoid using test class in the artifact name. Instead it now includes the run id (e.g. 7743208137), which can be used to look up the run in Github. Since runs can be repeated, it also includes the run number (e.g. 187) to make it unique.
  3. Add Abstract*$* to Surefire's test selector. In itself this will not cause any extra tests to execute, but it allows matching inherited @Nested tests.
  4. Add set -x before running junit.sh to log the actual command.
  5. (not required) Bump default number of splits to 10. Usually some multiple of 10x10 tests are run, because it helps define failure rate in percents.

https://issues.apache.org/jira/browse/HDDS-10264

How was this patch tested?

class = TestRootedOzoneContract, whose tests are defined as @Nested in the abstract parent class AbstractOzoneContractTest:
https://github.com/adoroszlai/ozone/actions/runs/7743208137

class = TestServiceInfoProvider$*, matches all nested tests in this concrete unit test:
https://github.com/adoroszlai/ozone/actions/runs/7743319908

class = TestMiniOzoneCluster, which is a regular integration test without any nested tests:
https://github.com/adoroszlai/ozone/actions/runs/7743334548

class = TestOMRatisSnapshots, method = testInstallIncrementalSnapshot, a specific method in a specific test class (without any nested tests):
https://github.com/adoroszlai/ozone/actions/runs/7743350631

class = TestServiceInfoProvider$UnsecureEnvironment: all methods in a nested test class:
https://github.com/adoroszlai/ozone/actions/runs/7743364840

class = TestLegacyReplicationManager$Misc, method = testGetContainerReplicaCount: specific method in a specific nested test class:
https://github.com/adoroszlai/ozone/actions/runs/7743385162

@adoroszlai adoroszlai added the CI label Feb 1, 2024
@adoroszlai adoroszlai self-assigned this Feb 1, 2024
@adoroszlai adoroszlai requested a review from sadanand48 February 1, 2024 18:12
Copy link
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adoroszlai for the patch , LGTM. Now that the checks are removed , it should also be possible to run multiple tests in a single job which wasn't possible earlier by giving a comma separated list.

@adoroszlai
Copy link
Contributor Author

Thanks @sadanand48 for the review.

Now that the checks are removed , it should also be possible to run multiple tests in a single job which wasn't possible earlier by giving a comma separated list.

Good point, now verified here:
https://github.com/adoroszlai/ozone/actions/runs/7752487414/job/21142133030#step:6:661

@adoroszlai adoroszlai merged commit 6732b64 into apache:master Feb 2, 2024
@adoroszlai adoroszlai deleted the HDDS-10264 branch February 2, 2024 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants